Skip to content

Disable integrations OAuth flow for billing/viewer roles#4008

Merged
steven-tey merged 3 commits into
mainfrom
disable-oauth-flow-for-billing-viewer-roles
Jun 12, 2026
Merged

Disable integrations OAuth flow for billing/viewer roles#4008
steven-tey merged 3 commits into
mainfrom
disable-oauth-flow-for-billing-viewer-roles

Conversation

@pepeladeira

@pepeladeira pepeladeira commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Enforced workspace-level permission checks when authorizing OAuth integrations; users without required permissions will be prevented from completing authorization.
    • Authorize button now disables when no valid workspace is selected or when workspace state is unresolved.
  • Bug Fixes

    • Improved tooltips and error messaging to clearly explain missing permissions, missing workspace selection, or plan restrictions when enabling or authorizing integrations.

@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dub Ready Ready Preview Jun 12, 2026 6:50pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51999ec6-d5ea-4c3f-8e89-821a4e2f2916

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4dd1a and cbe24e5.

📒 Files selected for processing (3)
  • apps/web/app/api/oauth/authorize/route.ts
  • apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/integrations/[integrationSlug]/page-client.tsx

📝 Walkthrough

Walkthrough

Backend OAuth POST route now requires workspace integrations.write permission; the frontend AuthorizeForm consolidates permission and scope checks into a memoized flow and disables/substantiates the Authorize button accordingly; integration settings tooltips now surface permission errors when applicable.

Changes

Workspace Permissions for OAuth and Integrations

Layer / File(s) Summary
Backend OAuth route permission enforcement
apps/web/app/api/oauth/authorize/route.ts
POST route is wrapped with withWorkspace(..., { requiredPermissions: ["integrations.write"] }) to enforce workspace-level permission checks while preserving existing OAuth parsing, scope/plan/redirect/PKCE logic, and callback URL response.
AuthorizeForm imports and permission check wiring
apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx
Adds useMemo and imports clientAccessCheck; consolidates permissionsError, missingScopes, and workspaceUnresolvedMessage derived from selected workspace, workspaces load state, and requested scopes.
AuthorizeForm submission guard and button UX updates
apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx
onAuthorize now ensures a resolved workspaceId before submitting; the Authorize button is disabled for no selection, unresolved workspace, permission errors, or missing scopes, with refined tooltip precedence.
Integration settings page permission error display
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/integrations/[integrationSlug]/page-client.tsx
Enable button disabledTooltip fallback now returns `permissionsError

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Suggested reviewers

  • steven-tey
  • devkiran

"A rabbit hops with joy and cheer,
guarding scopes both far and near.
Permissions checked and tooltips bright,
OAuth flows now snug and right.
🐰✨"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: restricting the OAuth integration authorization flow to roles with integrations.write permission, blocking billing and viewer roles.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch disable-oauth-flow-for-billing-viewer-roles

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pepeladeira pepeladeira marked this pull request as ready for review June 12, 2026 18:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/app.dub.co/`(auth)/oauth/authorize/authorize-form.tsx:
- Around line 34-40: The current useMemo treats a missing workspace as a
harmless clean state which allows form submission; update the logic in the
useMemo block (the computation that produces permissionsError and missingScopes)
to treat a selectedWorkspace that does not resolve to an actual workspace as a
not-yet-resolved/error state: if selectedWorkspace is set but workspaces is
undefined or workspace === undefined, return a non-empty permissionsError (or a
distinct flag) and ensure missingScopes stays empty so the UI disables the
submit button until the slug resolves to a real workspace entry; adjust any
consumers that check permissionsError/missingScopes (the submit enablement
logic) to rely on this new state.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 78fcdf4a-ed6e-4d3d-9760-6e37112e009f

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4dd1a and a35ac6d.

📒 Files selected for processing (3)
  • apps/web/app/api/oauth/authorize/route.ts
  • apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/integrations/[integrationSlug]/page-client.tsx

Comment thread apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx Outdated
@pepeladeira

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review. any breaking changes?

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== oauth authorize route ==="
cat apps/web/app/api/oauth/authorize/route.ts

echo ""
echo "=== authorize-form.tsx ==="
cat "apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx"

echo ""
echo "=== page-client.tsx ==="
cat "apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/integrations/[integrationSlug]/page-client.tsx"

echo ""
echo "=== clientAccessCheck implementation ==="
rg -rn "clientAccessCheck" --type=ts --type=tsx -B2 -A15 | head -100

echo ""
echo "=== withWorkspace options/types ==="
rg -rn "requiredPermissions" --type=ts -B2 -A5 | head -80

echo ""
echo "=== integrations.write permission ==="
rg -rn "integrations.write" --type=ts -B2 -A2 | head -60

echo ""
echo "=== Authorize GET route (to check if it also needs protection) ==="
cat apps/web/app/api/oauth/authorize/route.ts 2>/dev/null | head -20

echo ""
echo "=== GET route check ==="
rg -rn "export const GET" apps/web/app/api/oauth/authorize/route.ts

echo ""
echo "=== Roles and permissions config ==="
rg -rn "\"integrations.write\"" --type=ts -B2 -A2 | head -60

echo ""
echo "=== billing viewer role constants ==="
rg -rn "billing|viewer" apps/web/lib/role-permissions.ts 2>/dev/null | head -40
fd "role" apps/web/lib --type=f | head -10

Length of output: 37947


🏁 Script executed:

#!/bin/bash
echo "=== permissions.ts full ==="
cat apps/web/lib/api/rbac/permissions.ts

echo ""
echo "=== workspace-roles.ts ==="
cat apps/web/lib/workspace-roles.ts

echo ""
echo "=== clientAccessCheck full implementation ==="
fd "client-access-check" --type=f | xargs cat

echo ""
echo "=== withWorkspace WorkspaceOptions type ==="
rg -n "requiredPermissions" apps/web/lib/auth --type=ts -B2 -A5

echo ""
echo "=== scopes for role function ==="
rg -n "getScopesForRole" apps/web/lib/api/tokens/scopes.ts -A20

echo ""
echo "=== does the Authorize button also have the workspaceUnresolvedMessage in disabled? ==="
rg -n "workspaceUnresolvedMessage" "apps/web/app/app.dub.co/(auth)/oauth/authorize/authorize-form.tsx"

echo ""
echo "=== getIntegrationInstallUrl action - check for permissions ==="
fd "get-integration-install-url" --type=f | xargs cat 2>/dev/null

echo ""
echo "=== installAppsFlyerAction - check for permissions ==="
fd "install.ts" apps/web/lib/integrations/appsflyer --type=f | xargs cat 2>/dev/null

Length of output: 12576



@pepeladeira Here's the full review of PR #4008.


Summary

The PR adds both server-side (withWorkspace) and client-side (clientAccessCheck) enforcement of integrations.write permission, which is only granted to owner and member roles. billing and viewer roles will now be blocked from authorizing integrations via the OAuth flow.


Breaking Changes

Yes — intentional and expected:

Role Before After
owner ✅ Can authorize ✅ Can authorize
member ✅ Can authorize ✅ Can authorize
viewer ✅ Could authorize (no check) ❌ Blocked (UI + API)
billing ✅ Could authorize (no check) ❌ Blocked (UI + API)

This is the intended behavior. No owner/member workflows are disrupted.


Issues Found

🔴 Permission Enforcement Gap — installAppsFlyerAction

The apps/web/lib/integrations/appsflyer/install.ts server action only checks the plan, not the role:

// Only checks plan — no role/permission check
if (!getPlanCapabilities(workspace.plan).canInstallAdvancedIntegrations) {
  throw new Error("AppsFlyer integration is only available on Advanced and Enterprise plans.");
}

A billing or viewer user on an Advanced plan could bypass the UI and call this server action directly to install AppsFlyer. The getIntegrationInstallUrl action (for Slack/HubSpot) already has a role guard via throwIfNoPermission. AppsFlyer needs the same treatment:

import { throwIfNoPermission } from "`@/lib/actions/throw-if-no-permission`";

// Add after workspace is available from ctx:
throwIfNoPermission({
  role: workspace.role,
  requiredRoles: ["owner", "member"],
});

🟡 Inconsistent Permission Enforcement Pattern

getIntegrationInstallUrl uses a direct role check (requiredRoles: ["owner", "member"]) rather than the permission-based approach (integrations.write) used everywhere else in this PR. Consider aligning it to use throwIfNoPermission with the permission action for consistency and future-proofing.


🟢 Logic is Correct

  • authorize-form.tsx: The useMemo correctly short-circuits when !selectedWorkspace or while workspaces are loading. The disabled + disabledTooltip logic covers all states properly.
  • page-client.tsx: The disabledTooltip fallback chain (plan gate → permissionsError → undefined) is correct for both "Enable" and "Remove" buttons.
  • clientAccessCheck returns error: false (not undefined) when allowed — !!permissionsError handles this correctly in all call sites.
  • API route: requiredPermissions: ["integrations.write"] on the POST handler is the right layer to enforce this server-side.

TL;DR — The core change is solid with no unintended regressions for existing owner/member users. The main gap to address is installAppsFlyerAction missing the role permission check.

✅ Action performed

Full review finished.

@steven-tey steven-tey merged commit b55963d into main Jun 12, 2026
11 checks passed
@steven-tey steven-tey deleted the disable-oauth-flow-for-billing-viewer-roles branch June 12, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants